Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal #412

Merged
merged 7 commits into from
Oct 21, 2024
Merged

MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal #412

merged 7 commits into from
Oct 21, 2024

Conversation

mirafedas
Copy link
Contributor

@mirafedas mirafedas commented Sep 23, 2024 •
edited
Loading

MWPW-158911:
In merch-sidenav.js, when the filters modal is closed by clicking outside of it (rather than using the 'Close' button), the overlay emits a close event. On this event, we need to remove the merch-modal class; otherwise, the page will remain unscrollable.
!!! The related Milo PR has to be merged first for this feature to work: adobecom/milo#2933

MWPW-159427:
On iOS version 15 and lower there is a different way of managing the vertical stacking context, which puts the modal below the merch card. Adding z-index on the merch-sidenav resolves this problem.
Related Milo PR: adobecom/milo#2994

Resolves: MWPW-158911 and MWPW-159427

Test URLs for MWPW-158911:

Test URLs for MWPW-159427:

For PSI check:
https://mwpw-158911-modal-scroll--cc--adobecom.hlx.live/products/animate/free-trial-download?martech=off&georouting=off

@mirafedas mirafedas requested a review from a team as a code owner September 23, 2024 10:08
@aem-code-sync AEM Code Sync
Copy link

aem-code-sync bot commented Sep 23, 2024 •
edited
Loading

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

@aem-code-sync AEM Code Sync
Copy link

aem-code-sync bot commented Sep 23, 2024 •
edited
Loading

Page Scores Audits Google
📱 /products/animate/free-trial-download?martech=off&georouting=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
🖥️ /products/animate/free-trial-download?martech=off&georouting=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@yesil
Copy link
Contributor

yesil commented Sep 23, 2024

@mirafedas can you link the Milo PR?

@mirafedas mirafedas mentioned this pull request Sep 23, 2024
Merged
@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to failing checks

@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to missing verified label. kindly make sure that the PR has been verified

2 similar comments
@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to missing verified label. kindly make sure that the PR has been verified

@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to missing verified label. kindly make sure that the PR has been verified

@aem-code-sync aem-code-sync bot temporarily deployed to mwpw-158911-modal-scroll September 26, 2024 07:54 Inactive
@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to failing checks

@aem-code-sync aem-code-sync bot temporarily deployed to mwpw-158911-modal-scroll September 26, 2024 10:28 Inactive
@aem-code-sync aem-code-sync bot temporarily deployed to mwpw-158911-modal-scroll September 27, 2024 04:14 Inactive
@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to failing checks

@aem-code-sync aem-code-sync bot temporarily deployed to mwpw-158911-modal-scroll September 30, 2024 11:14 Inactive
@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to missing verified label. kindly make sure that the PR has been verified

1 similar comment
@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to missing verified label. kindly make sure that the PR has been verified

Axelcureno
Axelcureno reviewed Oct 2, 2024
View reviewed changes
README.md Outdated Show resolved Hide resolved
@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to missing verified label. kindly make sure that the PR has been verified

@aem-code-sync aem-code-sync bot temporarily deployed to mwpw-158911-modal-scroll October 3, 2024 09:26 Inactive
@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to missing verified label. kindly make sure that the PR has been verified

@aem-code-sync aem-code-sync bot temporarily deployed to mwpw-158911-modal-scroll October 8, 2024 10:30 Inactive
@aem-code-sync aem-code-sync bot temporarily deployed to mwpw-158911-modal-scroll October 8, 2024 11:43 Inactive
github-actions[bot]
github-actions bot reviewed Oct 8, 2024
View reviewed changes
creativecloud/blocks/sidenav/sidenav.js Outdated Show resolved Hide resolved
creativecloud/blocks/sidenav/sidenav.js Outdated Show resolved Hide resolved
@aem-code-sync aem-code-sync bot temporarily deployed to mwpw-158911-modal-scroll October 8, 2024 11:50 Inactive
@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to failing checks

@aem-code-sync aem-code-sync bot temporarily deployed to mwpw-158911-modal-scroll October 8, 2024 13:56 Inactive
@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to failing checks

@mirafedas mirafedas requested a review from yesil October 9, 2024 14:39
@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to missing verified label. kindly make sure that the PR has been verified

2 similar comments
@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to missing verified label. kindly make sure that the PR has been verified

@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to missing verified label. kindly make sure that the PR has been verified

@mirafedas
Copy link
Contributor Author

@yesil I added the change to use the merch-sidenav.js directly from Milo, could you please review?

@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to missing verified label. kindly make sure that the PR has been verified

1 similar comment
@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to missing verified label. kindly make sure that the PR has been verified

@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to missing verified label. kindly make sure that the PR has been verified

@yesil yesil mentioned this pull request Oct 18, 2024
Closed
@milo-pr-merge-cc
Copy link

Skipped merging 412: MWPW-158911: Unable to scroll page if the filters modal closed by click outside of the modal due to missing verified label. kindly make sure that the PR has been verified

@milo-pr-merge-cc milo-pr-merge-cc bot merged commit 491ef09 into stage Oct 21, 2024
12 checks passed
@milo-pr-merge-cc milo-pr-merge-cc bot mentioned this pull request Oct 21, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@yesil yesil yesil approved these changes

@npeltier npeltier npeltier approved these changes

@rahulgupta999 rahulgupta999 Awaiting requested review from rahulgupta999

@3ch023 3ch023 Awaiting requested review from 3ch023

@rohitsahu rohitsahu Awaiting requested review from rohitsahu

@bozojovicic bozojovicic Awaiting requested review from bozojovicic

@seanchoi-dev seanchoi-dev Awaiting requested review from seanchoi-dev

@Axelcureno Axelcureno Awaiting requested review from Axelcureno

Assignees
No one assigned
Labels
ready for stage verified
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mirafedas @yesil @Roycethan @npeltier @Axelcureno